Skip to content

invoke async function as IIFE in example#958

Closed
avindra wants to merge 1 commit intotediousjs:masterfrom
avindra:patch-1
Closed

invoke async function as IIFE in example#958
avindra wants to merge 1 commit intotediousjs:masterfrom
avindra:patch-1

Conversation

@avindra
Copy link

@avindra avindra commented Dec 11, 2019

The example as listed currently does not run anything, so wrapping it up as an async IIFE.

What this does:

Make sure users who are copy+pasting the example in the README can get it to run

The example as listed currently does not run anything, so wrapping it up as an async IIFE.
@dhensby
Copy link
Collaborator

dhensby commented Dec 13, 2019

Thanks for the contribution @avindra

I'm not entirely sure this change makes the docs any more helpful - it's not intended to be a copy+paste example of getting a query to run, it's meant to be an abstract example of how you would go about writing code. I think putting the example code in an IIFE will actually make more people's code break as they'll copy+paste it into their functions and not understand why it's running when the function hasn't been called.

@willmorgan
Copy link
Collaborator

why it's running when the function hasn't been called

It's been called in the IIFE!

@dhensby
Copy link
Collaborator

dhensby commented Dec 13, 2019

It's been called in the IIFE!

Yes, but is someone blindingly copy and pasting code going to understand that? these examples are not for blind copy and paste to get your application working, they are conceptual examples about how to make a request - if you can't figure out you need to actually call that code for it to run, I don't think they'll understand why it's running inside an IIFE

@dhensby
Copy link
Collaborator

dhensby commented Dec 13, 2019

If we do think this is how our docs should be written then every single example needs updating, right?

@avindra
Copy link
Author

avindra commented Dec 13, 2019

Fair points, gents. I was able to get up and running from the example, so I'll leave it be for now.

As you mentioned @dhensby everything else should get updated if this is the approach.

I'm closing this for now... may re-open it later on if I get some time to update the rest of the docs

@avindra avindra closed this Dec 13, 2019
@dhensby
Copy link
Collaborator

dhensby commented Apr 2, 2020

@dhensby dhensby mentioned this pull request Jul 22, 2020
@avindra
Copy link
Author

avindra commented Oct 2, 2020

@dhensby good points are raised in that anti-iife article. Top level await is available (behind a flag) in node 14.3.0 and is already in deno. At some point the function can be removed, and the top level await should work as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants